src/main/resources/templates/login/login.html (1)

24-26: Optional: surface retryAfter in the lockout message.

LoginLockoutFilter already redirects with retryAfter=<seconds> (per the filter test). Consider showing the remaining time so users understand how long to wait, e.g.:

<div th:if="${param.locked}" class="panel panel-error" style="margin-bottom: 14px;">
    Too many login attempts. Please try again in
    <span th:text="${param.retryAfter != null ? param.retryAfter + ' seconds' : 'a few minutes'}">a few minutes</span>.
</div>

Functional as-is; this is purely a UX polish.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/templates/login/login.html` around lines 24 - 26, Update
the login template to display the lockout remaining time provided by
LoginLockoutFilter by reading the retryAfter request param instead of the static
phrase; inside the existing th:if="${param.locked}" block use param.retryAfter
(e.g. show "${param.retryAfter} seconds" when present, fallback to the current
"a few minutes" text) so users see the actual wait time returned by
LoginLockoutFilter.

src/test/java/org/example/projektarendehantering/infrastructure/security/LoginLockoutFilterTest.java (1)

12-50: Consider additional coverage for non-matching requests.

Current tests cover only POST /login paths. To harden the filter's request matching, consider adding cases that exercise:

• GET /login (should bypass the lock check),
• requests to other paths (e.g. /register, /api/…) that should not be intercepted,
• missing username parameter (verify no NPE and expected behavior).

Not a blocker — the happy paths are well-tested.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/infrastructure/security/LoginLockoutFilterTest.java`
around lines 12 - 50, Add tests to LoginLockoutFilterTest that exercise
non-matching requests: create cases invoking LoginLockoutFilter.doFilter with
(1) a GET "/login" request (MockHttpServletRequest with method "GET") and assert
the filter does not call redirect and the chain proceeded, (2) a POST to a
different path like "/register" or "/api/foo" and assert the same, and (3) a
POST "/login" with no "username" parameter to verify no NPE and that the request
is passed through; in each test mock LoginAttemptService.currentLockDecision
only if expected to be called (and verify it is not called for non-matching
requests) and use MockFilterChain/MockHttpServletResponse assertions
(response.getRedirectedUrl() == null, response.getStatus()==200 or chain
proceeded) to confirm correct behavior.

src/main/java/org/example/projektarendehantering/infrastructure/security/LoginLockoutFilter.java (1)

47-48: Nit: URLEncoder.encode on a numeric retryAfter is unnecessary.

decision.retryAfterSeconds() is a long/int; its decimal representation is URL-safe. The encoding adds no protection and marginally obscures the value in logs. Inlining decision.retryAfterSeconds() directly is cleaner (same applies in LoginAuthenticationFailureHandler Line 38).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/infrastructure/security/LoginLockoutFilter.java`
around lines 47 - 48, The redirect currently URL-encodes a numeric retryAfter
value unnecessarily; in LoginLockoutFilter replace the URLEncoder.encode usage
and append decision.retryAfterSeconds() directly (or its String form) when
building the "/login?error=true&locked=true&retryAfter=" redirect, and make the
same change in LoginAuthenticationFailureHandler at the analogous location (line
~38) so the numeric seconds are inlined without encoding.

src/test/java/org/example/projektarendehantering/infrastructure/security/RateLimitServiceTest.java (1)

58-79: Optional: extract MutableClock into a shared test helper.

The same private MutableClock is reintroduced verbatim in LoginAttemptServiceTest. Consider moving it to a package-private test utility (e.g., infrastructure/security/testing/MutableClock) to avoid drift if the helper evolves.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/infrastructure/security/RateLimitServiceTest.java`
around lines 58 - 79, Extract the private MutableClock inner class used in
RateLimitServiceTest into a shared package-private test utility so
LoginAttemptServiceTest can reuse it; specifically, create a new test helper
class named MutableClock (matching the current constructor and overrides:
instant(), getZone(), withZone()) in a shared test package (e.g.,
infrastructure.security.testing) and replace the private inner MutableClock in
both RateLimitServiceTest and LoginAttemptServiceTest with imports of that
shared MutableClock. Ensure the new helper preserves the same API (constructor
taking Instant and the three overridden methods) and visibility so both tests
can access it.

src/test/java/org/example/projektarendehantering/infrastructure/security/LoginAuthenticationHandlersTest.java (1)

32-47: Optional: add a case that exercises a non-canonical username.

Both interactions use "user@example.com" verbatim, so the test cannot detect inconsistent normalization between recordFailure(request.getParameter("username"), ...) and recordSuccess(authentication.getName(), ...) (see the verification comment on LoginAuthenticationSuccessHandler). Consider a case where the form field is " User@Example.com " while the authenticated principal is "user@example.com" to assert the service key is consistent.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/infrastructure/security/LoginAuthenticationHandlersTest.java`
around lines 32 - 47, Add a new test that verifies normalization consistency
between the submitted username and the authentication principal: create a
LoginAuthenticationSuccessHandler with a mocked LoginAttemptService, simulate a
request whose form username parameter is a non-canonical value (e.g., " 
User@Example.com "), set request remoteAddr, build an Authentication whose
getName() returns the canonical "user@example.com", call
handler.onAuthenticationSuccess(request, response, authentication), and verify
LoginAttemptService.recordSuccess was called with the canonical username
("user@example.com") and the request IP; ensure the response redirect assertion
("/home") remains.

src/test/java/org/example/projektarendehantering/infrastructure/security/LoginAttemptServiceTest.java (1)

13-51: LGTM — consider optional coverage for per-IP vs per-user keying.

Solid tests for threshold, TTL expiry, and success-clears behaviour. If the service keys lockouts per (username, ip) pair (or per either), add a test asserting that failures from a different IP don't lock the same user, and/or that failures against different usernames from the same IP behave as expected. This tends to be the surface where brute-force bypasses creep in.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/infrastructure/security/LoginAttemptServiceTest.java`
around lines 13 - 51, Add tests to verify whether LoginAttemptService keys by
(username, ip) pair or by single dimension: create new test(s) that use
LoginAttemptService (constructor used in existing tests) and call recordFailure
for "user@example.com" from "127.0.0.1" enough times to hit the threshold, then
assert currentLockDecision("user@example.com","127.0.0.2").locked() is false
(different IP) and/or assert
currentLockDecision("other@example.com","127.0.0.1").locked() is false
(different user); also add the inverse case if needed to document behavior.
Target the same class and methods: LoginAttemptService.recordFailure,
LoginAttemptService.currentLockDecision, and LoginAttemptService.recordSuccess
to show expected per-IP vs per-user semantics.

src/main/resources/static/app.js (2)

110-133: Consider throttling syncActiveSlideFromViewport.

The listener runs on every scroll event — including the scroll events emitted during a programmatic scrollIntoView from goToSlide — and each call does slides.length × getBoundingClientRect() (which forces a layout). For a handful of slides it's fine, but requestAnimationFrame-coalescing (or a simple 16ms throttle) keeps this off the main thread during inertial scrolling and avoids the brief flicker of active-link state during smooth-scroll animations.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/app.js` around lines 110 - 133, Thottle scroll
handler by coalescing syncActiveSlideFromViewport calls with
requestAnimationFrame: add a module-level flag (e.g., isTicking) and in the
scroll listener (the window.addEventListener("scroll", ...)) only schedule
syncActiveSlideFromViewport via requestAnimationFrame when not already
scheduled, clearing the flag inside syncActiveSlideFromViewport after work
completes; ensure the listener remains passive and keep references to
slides/getBoundingClientRect logic as-is so goToSlide-triggered programmatic
scrolls are coalesced and avoid repeated forced layouts.

---

302-310: Simplify initial hash handling.

The first goToSlide(..., { shouldScroll: false }) is used only to sync visual state, then a second call with shouldScroll: true performs the actual scroll. Since goToSlide already runs syncBySlide() before scrolling, a single call covers both paths:

♻️ Proposed simplification

-  const hash = window.location.hash.replace("#", "");
-  const initialIndex = hash ? slideIds.indexOf(hash) : 0;
-  activeSlideIndex = initialIndex >= 0 ? initialIndex : 0;
-  goToSlide(activeSlideIndex, { behavior: "auto", shouldScroll: false, updateHash: false });
-  if (hash) {
-    goToSlide(activeSlideIndex, { behavior: "auto", shouldScroll: true, updateHash: false });
-  } else {
-    updateProgress();
-  }
+  const hash = window.location.hash.replace("#", "");
+  const initialIndex = hash ? slideIds.indexOf(hash) : 0;
+  const startIndex = initialIndex >= 0 ? initialIndex : 0;
+  goToSlide(startIndex, { behavior: "auto", shouldScroll: Boolean(hash), updateHash: false });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/app.js` around lines 302 - 310, Replace the two
goToSlide calls and the if/else block with a single call: determine hash and
initialIndex as before, set activeSlideIndex, then call
goToSlide(activeSlideIndex, { behavior: "auto", shouldScroll: !!hash,
updateHash: false }); and remove the redundant first call and the separate
conditional branch (including the alternate updateProgress() call) since
goToSlide runs syncBySlide() and will handle progress/scrolling appropriately.

src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java (1)

69-70: Chain the second filter relative to the first to make the execution order explicit.

Both filters are added before UsernamePasswordAuthenticationFilter. While modern Spring Security (6.x+) handles filter ordering correctly, relying on insertion order for implicit sequencing is unclear. Spring Security's documentation recommends chaining filter positions explicitly when order matters.

Make the intended RateLimitFilter → LoginLockoutFilter → UsernamePasswordAuthenticationFilter pipeline unambiguous:

Suggested refactor

-            .addFilterBefore(rateLimitFilter, UsernamePasswordAuthenticationFilter.class)
-            .addFilterBefore(loginLockoutFilter, UsernamePasswordAuthenticationFilter.class);
+            .addFilterBefore(rateLimitFilter, UsernamePasswordAuthenticationFilter.class)
+            .addFilterAfter(loginLockoutFilter, RateLimitFilter.class);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java`
around lines 69 - 70, The two filters (rateLimitFilter and loginLockoutFilter)
are both added before UsernamePasswordAuthenticationFilter which leaves their
relative order implicit; update SecurityConfig so the rate limit filter is
explicitly placed before the login lockout filter and the login lockout filter
is placed before UsernamePasswordAuthenticationFilter to enforce RateLimitFilter
→ LoginLockoutFilter → UsernamePasswordAuthenticationFilter. Concretely, call
addFilterBefore(rateLimitFilter, LoginLockoutFilter.class) and then
addFilterBefore(loginLockoutFilter, UsernamePasswordAuthenticationFilter.class)
(using the actual filter implementation classes for LoginLockoutFilter) in the
SecurityConfig configuration where the filters are registered.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes